I'm not sure what 173~176 do. ExceptionWrapper.php does not have a variable named $exception, and at this point you already have the thing you need to log.

Is this leftover test flow or something like that?

removed the redundant code.

Question:

• Should we attach every exception, not just the buried one? It feels strange to see a test fail in two steps but only see one Exception attached. We could even label it Before/Test/After Exception using some functions in this class already.

Those were my initial thoughts too, but looks like After block's exception stack trace appears in the allure report twice - one at the top and another at the bottom after test steps. Will it be a duplication in some way? Capturing context and appending it to attachment name would work, however, there will only be 1 exception in Before + Test body scope, would context be redundant to display? Also looks like Allure doesn't show clear separation of Before/Test/After blocks of the test. What are your thoughts?

So good news is that with just Codeception 2.4.0 we can do a couple things:

• Stop iterating over errors on line 88, they will only now be parent->previous() relationship
• Given above, we can instead iterate by doing logError($error), wherein we log the parent, check for child and call logError($childError) to pop the full stack
• remove the testFail() hook entirely, just use testEnd() to log these

We can modify the hook to look something like:

...
$testResultObject = call_user_func(\Closure::bind(
    function () use ($cest) {
        return $cest->getTestResultObject();
    },
    $cest
));
if (!empty($testResultObject->errors())) {
    foreach ($testResultObject->errors() as $error) {
        $this->attachExceptionToAllure($error->thrownException());
    }
}
if (!empty($testResultObject->failures())) {
    foreach ($testResultObject->failures() as $error) {
        $this->attachExceptionToAllure($error->thrownException());
    }
}
...

We can end up with something like this

You could add to the signature of attachExceptionToAllure to inlcude a prefix like Error vs Failure as well for clearer deliniation.

Even if we have some duplication, I think if we lean on prefixing the attachments it would be best in case someone needs the info. Those can be ignored by simply not expanding them.

Added context as a prefix to the attachment. Including a prefix like Error or Failure may need more investigation. From what I understood, PHPUnit\Framework\Exception are marked as failures and Magento\Framework\Exception are marked as Errors. Maybe there are exceptions which don't follow this criteria.

I think you are right in not trusting that the exception types are necessarily linked to failure vs error. You could include them in the signature of your new attachExceptionToAllure but I think having the prefix is enough for now.